86b883b71 - Add Numeric Validation for Quantities Fields#761
Merged
86b883b71 - Add Numeric Validation for Quantities Fields#761
Conversation
ghost
requested a review
from 
ghost
assigned
📝 WalkthroughWalkthroughAdds a new reusable Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/mui/formik-inputs/mui-formik-quantity-field.js`:
- Around line 7-19: The onKeyDown filter in MuiFormikQuantityField prevents
blocked keys (BLOCKED_KEYS) but paste and drop can still inject
scientific-notation characters; update the MuiFormikQuantityField props passed
to MuiFormikTextField to add onPaste and onDrop handlers that inspect the
pasted/dropped string and if it contains any BLOCKED_KEYS characters or
scientific-notation patterns (e or E with optional sign) call preventDefault()
and stopImmediatePropagation(), otherwise allow the input; reference the
component name MuiFormikQuantityField, the child MuiFormikTextField and the
BLOCKED_KEYS constant when making the change.
In `@src/utils/yup.js`:
- Around line 56-61: The quantityValidation() schema currently uses
yup.number().positive() which rejects 0 and allows decimals, creating a mismatch
with the UI (min: 0) and other validators; replace its implementation to use the
shared positiveNumberValidation() helper (the same used by
quantity_limit_per_sponsor and quantity_limit_per_show) so it enforces integer
and min(0) semantics consistent with the UI and other validators; locate and
swap the body of quantityValidation to delegate to positiveNumberValidation() or
mirror its .integer().min(0) rules and reuse the existing translation keys for
error messages.
♻️ Duplicate comments (1)
src/pages/sponsors/sponsor-form-item-list-page/components/sponsor-form-item-form.js (1)
36-48: Use ofquantityValidationinherits the validator semantics.See the note on
quantityValidationinsrc/utils/yup.js; the same 0/decimal vs integer concern applies here.
🧹 Nitpick comments (1)
src/pages/sponsors/sponsor-form-item-list-page/components/sponsor-form-item-form.js (1)
124-155: Avoid overridingMuiFormikQuantityFielddefaults.Passing
typeandinputPropshere overrides the component’sinputModeand duplicates its own defaults. Consider relying on the wrapper or explicitly merging inputProps.♻️ Streamlined usage
<MuiFormikQuantityField name="quantity_limit_per_show" label={T.translate( "sponsor_form_item_list.edit_item.quantity_limit_per_show" )} fullWidth - type="number" - inputProps={{ min: 0 }} /> ... <MuiFormikQuantityField name="quantity_limit_per_sponsor" label={T.translate( "sponsor_form_item_list.edit_item.quantity_limit_per_sponsor" )} fullWidth - type="number" - inputProps={{ min: 0 }} /> ... <MuiFormikQuantityField name="default_quantity" label={T.translate( "sponsor_form_item_list.edit_item.default_quantity" )} fullWidth - type="number" - inputProps={{ min: 0 }} required />
smarcet
reviewed
Jan 22, 2026
src/pages/sponsors/sponsor-form-item-list-page/components/sponsor-form-item-form.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/mui/formik-inputs/mui-formik-quantity-field.js`:
- Around line 16-21: The component currently spreads {...props} after passing
inputProps={{ min: 0, inputMode: "numeric" }}, which allows a consumer-provided
inputProps to fully overwrite those defaults; update the component to
destructure inputProps from props (e.g., const { inputProps: userInputProps,
...rest } = props) and merge them with the defaults (preserving min: 0 and
inputMode: "numeric") before spreading the rest of props and the merged
inputProps into the input; reference the existing inputProps usage and the props
spread in mui-formik-quantity-field.js to locate where to perform the merge.
🧹 Nitpick comments (2)
src/components/mui/formik-inputs/mui-formik-quantity-field.js (2)
5-5: Consider blocking the decimal point for integer quantities.The current
BLOCKED_KEYSprevents scientific notation and signs, but still allows.for decimal input. If quantities should be integers (which is typical for inventory counts), consider adding"."to the blocked keys.🔧 Suggested change
-const BLOCKED_KEYS = ["e", "E", "+", "-"]; +const BLOCKED_KEYS = ["e", "E", "+", "-", "."];
10-14: Simplify to use synthetic event'spreventDefault().Using
e.nativeEvent.preventDefault()andstopImmediatePropagation()is unconventional. For blocking key input,e.preventDefault()on the synthetic event is sufficient—stopImmediatePropagation()is unnecessary here.♻️ Suggested simplification
onKeyDown={(e) => { if (BLOCKED_KEYS.includes(e.key)) { - e.nativeEvent.preventDefault(); - e.nativeEvent.stopImmediatePropagation(); + e.preventDefault(); } }}
Comment on lines
+16
to
+21
| inputProps={{ | ||
| min: 0, | ||
| inputMode: "numeric" | ||
| }} | ||
| // eslint-disable-next-line react/jsx-props-no-spreading | ||
| {...props} |
There was a problem hiding this comment.
Props spread can silently override inputProps, losing min and inputMode.
If a consumer passes inputProps={{ max: 100 }}, it will completely replace the component's inputProps, losing min: 0 and inputMode: "numeric". Consider destructuring inputProps and merging it.
🔧 Suggested fix
-const MuiFormikQuantityField = ({ ...props }) => (
+const MuiFormikQuantityField = ({ inputProps: customInputProps, ...props }) => (
<MuiFormikTextField
type="number"
onKeyDown={(e) => {
if (BLOCKED_KEYS.includes(e.key)) {
- e.nativeEvent.preventDefault();
- e.nativeEvent.stopImmediatePropagation();
+ e.preventDefault();
}
}}
inputProps={{
min: 0,
- inputMode: "numeric"
+ inputMode: "numeric",
+ ...customInputProps
}}
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
/>
);🤖 Prompt for AI Agents
In `@src/components/mui/formik-inputs/mui-formik-quantity-field.js` around lines
16 - 21, The component currently spreads {...props} after passing inputProps={{
min: 0, inputMode: "numeric" }}, which allows a consumer-provided inputProps to
fully overwrite those defaults; update the component to destructure inputProps
from props (e.g., const { inputProps: userInputProps, ...rest } = props) and
merge them with the defaults (preserving min: 0 and inputMode: "numeric") before
spreading the rest of props and the merged inputProps into the input; reference
the existing inputProps usage and the props spread in
mui-formik-quantity-field.js to locate where to perform the merge.
Author
|
Ready for review! |
Author
|
@romanetar Don't forget to review this PR! |
romanetar
deleted the
feature/add_numeric_validation_for_quantities_fields
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ref: https://app.clickup.com/t/86b883b71
86b883b71 - Add Numeric Validation for Quantities Fields
Changelog
Links
86b883b71 - Add Numeric Validation for Quantities Fields
Evidence
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.